fix(zabbix): populate hostname on pulled and pushed alerts#6612
fix(zabbix): populate hostname on pulled and pushed alerts#6612washosk wants to merge 4 commits into
Conversation
Zabbix problem.get() alerts never carried a host, since _get_alerts()
never requested host data from the API. Add selectHosts to the request
and map it onto AlertDto's service/hostname fields.
The webhook push path had a separate bug: the installed script sends
the host as host_name/host_ip (matching its own parameter names), but
_format_alert() looked for hostName/hostIp, so the fallback silently
returned None whenever {HOST.HOST} was empty (e.g. multi-host trigger
problems).
dosubot
Bot
added
size:S
|
|
shahargl
left a comment
shahargl
left a comment
There was a problem hiding this comment.
Can you just add the new fields and not remove the old one? For backward compatibility /older zabbix versions
Per review: don't drop the hostName/hostIp lookups, add host_name/ host_ip alongside them. Also restore HOST.NAME/HOST.IP, the raw macro keys used by the pre-script-based (mediatype) webhook installs that predate keephq#2397 and may still be configured on older Zabbix instances.
|
Good catch pushed a fix that keeps hostName/hostIp (and the even older HOST.NAME/HOST.IP macro keys from the pre-script mediatype webhook) as fallbacks alongside host_name/host_ip, so older installs that haven't re-run the webhook setup keep working. |
selectHosts is not a valid problem.get parameter -- Zabbix rejects it with "Invalid parameter: unexpected parameter selectHosts", breaking every poll (confirmed against a live production instance). Fetch hosts via a separate batched event.get call on the same eventids and join them back in by eventid instead. Requires the event.get scope.
dosubot
Bot
added
size:M
|
Correction to the earlier commit: problem.get does not support selectHosts (verified against a live instance — Zabbix rejects it with "Invalid parameter: unexpected parameter selectHosts", which broke every poll after deploying this branch). selectHosts is an event.get parameter, not problem.get. Pushed a fix: _get_alerts now does a second batched event.get call on the fetched eventids (with selectHosts) and joins the hostname back in, instead of asking problem.get for something it doesn't support. This also adds the event.get scope requirement. |
problem.get never returns a "status" text field, so the previous
STATUS_MAP.get(problem.pop("status", ""), ...) always fell through to
FIRING regardless of the problem's real state. Derive status from the
acknowledged/suppressed flags problem.get actually returns instead,
matching the pattern other providers (Datadog, PagerDuty, Grafana...)
already use to report these states from pull. problem.get only
returns open problems, so resolved/ok stay unreachable here (as
before).
Verified against real production problems: suppressed=1 now yields
SUPPRESSED, acknowledged=1 yields ACKNOWLEDGED, instead of both
showing as FIRING.
|
Note: rolled the status bug I mentioned above into this same PR instead of a separate issue, since it's the same _get_alerts loop. problem.get never returns a "status" text field (that's a UI-only concept), so the old STATUS_MAP.get(problem.pop("status", ""), AlertStatus.FIRING) always fell through to FIRING no matter the real state. Now deriving status from the acknowledged/suppressed flags problem.get actually returns — same approach Datadog/PagerDuty/Grafana providers already use to report these states on pull. Verified against real production problems: a suppressed=1 problem now correctly comes back SUPPRESSED instead of FIRING. |
4 participants
Fixes #6613
Summary
Zabbix alerts were missing the host name in two separate ways:
_get_alerts): theproblem.getcall never asked Zabbix for host data, so pulled alerts had no host information at all._format_alert): the installed Zabbix script sends the host ashost_name/host_ip(matching the parameter names configured insetup_webhook), but the code readhostName/hostIpinstead. The primary source,service({HOST.HOST}), can be empty for problems tied to multiple hosts, and the fallback silently returnedNonedue to this key mismatch.Changes
_get_alerts:problem.getdoes not supportselectHosts(verified against a live instance — it errors withInvalid parameter: unexpected parameter selectHosts). Fetch hosts via a separate batchedevent.getcall on the same eventids and join them back in by eventid, mapping the host's visible name (falling back to its technical name) ontoservice/hostnameon theAlertDto. Adds theevent.getscope._format_alert: readhost_name/host_ip(matching the actual webhook payload keys), falling back throughhostName/hostIpand the olderHOST.NAME/HOST.IPmacro keys for backward compatibility with webhook installs set up before the mediatype→script migration (chore(provider): zabbix from mediatype to script #2397).Note (added while re-testing against a live instance): also fixes an unrelated pre-existing bug in the same
_get_alertsloop:problem.getnever actually returns astatustext field (that's a UI concept, not an API field), soSTATUS_MAP.get(problem.pop("status", ""), AlertStatus.FIRING)always fell through toFIRINGregardless of the problem's real state. Status is now derived from theacknowledged/suppressedflagsproblem.getdoes return, the same pattern already used by other providers (Datadog, PagerDuty, Grafana, ...) to report these states from a pull.problem.getonly returns still-open problems, soresolved/okremain unreachable here, same as before.Test plan
problem.get+event.getjoin now returns real hostnames for real open problems (confirmed several, e.g. hostke-mba-icolo02-fw01afor an OPNsense DHCP problem).suppressed/acknowledgedflags on real problems now map toSUPPRESSED/ACKNOWLEDGEDinstead of alwaysFIRING._get_alertsand_format_alertcovering these cases